Skip to content

Staging: destructor-hazard-section deferral mechanism#154

Closed
iliaal wants to merge 1 commit into
masterfrom
proto/dtor-hazard-pin
Closed

Staging: destructor-hazard-section deferral mechanism#154
iliaal wants to merge 1 commit into
masterfrom
proto/dtor-hazard-pin

Conversation

@iliaal

@iliaal iliaal commented Jul 11, 2026

Copy link
Copy Markdown
Owner

Phase 2 of the destructor-reentrancy work, building on the error-handler deferral in 22515. Adds ZEND_DTOR_HAZARD_BEGIN/END sections and a deferral gate in zend_objects_store_del: while a section is open, an object that hits refcount zero and has a destructor is pinned and its destructor deferred until the section closes. Applied to three reentrancy use-after-frees where an internal C loop drops a refcount while holding a raw pointer a synchronous __destruct could free: concat (phpGH-20477), zend_hash_clean (phpGH-22061), and unserialize teardown (phpGH-21824), with a regression test for the concat-assign case (phpGH-19999); that test expects NULL because the deferred destructor runs at section exit, after the concat writes the result, and nulls $GLOBALS['a'].

How this differs from arnaud-lb#31: that prototype deferred every destructor through a single global EG(delayed_effects) queue, pinned the dying object with GC_SET_REFCOUNT(obj, 1), and flushed at the VM function-leave helper. It left three problems open: throwing destructors reordered against try/finally and catch (6 XFAILed tests), 67 tests needed a dummy call to create a flush point (a timing change), and GC reconciliation between the RC=1 pin and cycle collection stayed unresolved. This mechanism is scoped instead. BEGIN/END bracket only the specific internal loops that hold the raw pointer, so no user code runs inside a window. That drops the try/finally reordering question (no exception crosses a boundary), drops the timing change (flush is at section exit, not an arbitrary safepoint), uses GC_ADDREF instead of GC_SET_REFCOUNT so it reuses the existing store_del invariants, and closes the GC hole by having gc_possible_root_when_full skip synchronous collection while a section is open.

Performance: the store_del gate is one branch guarded by has_dtor, so destructor-less objects are untouched. Sections arm only on cold paths (concat arms only when an operand needed stringification or the result aliases an operand; hash_clean only when the table has a destructor). Release callgrind over 2M concats shows +1.2 instructions per concat, 0.45% of concat_function, with the common in-place concat paying one predicted-not-taken branch. No global vm_interrupt arming, no JIT may_throw change.

An internal C operation that drops a refcount while holding a raw pointer
into a structure a synchronous __destruct could free is a use-after-free.
Add ZEND_DTOR_HAZARD_BEGIN/END sections and a deferral gate in
zend_objects_store_del: while a section is open, an object that reaches
refcount zero and has a destructor is pinned with GC_ADDREF and its
destructor deferred until the section closes. gc_possible_root_when_full
skips synchronous collection while a section is open. Apply the sections
to concat (phpGH-20477), zend_hash_clean (phpGH-22061), and unserialize
teardown (phpGH-21824).

Fixes phpGH-20477
Fixes phpGH-22061
Fixes phpGH-21824
@iliaal iliaal force-pushed the proto/dtor-hazard-pin branch from 092b675 to d617101 Compare July 11, 2026 12:29
@iliaal

iliaal commented Jul 11, 2026

Copy link
Copy Markdown
Owner Author

Submitted upstream as php#22697.

@iliaal iliaal closed this Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant